Skip to content

Use assert_implements in test the-input-element/type-change-state.html #22145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 24, 2020
Merged

Use assert_implements in test the-input-element/type-change-state.html #22145

merged 1 commit into from
Apr 24, 2020

Conversation

clopez
Copy link
Contributor

@clopez clopez commented Mar 9, 2020

Use assert_precondition to check if the browser supports both input element types before trying to change the state from one to other.

For example, WebKitGTK and Safari doesn't support several of the input types tested here (like those related to dates). The output of the test its clearer if the precondition its checked before.

@clopez
Copy link
Contributor Author

clopez commented Mar 9, 2020

Check how it looks on safari before and after this https://wpt.fyi/results/html/semantics/forms/the-input-element/type-change-state.html?diff&filter=ADC&run_id=444340020&run_id=444320020 (the precondition_failed makes it clear to see the results and the actual failures)

@clopez clopez requested a review from stephenmcgruer March 30, 2020 21:12
@stephenmcgruer
Copy link
Contributor

Are these areas of the spec OPTIONAL? See web-platform-tests/rfcs#48 for an in-progress RFC where we intend to deprecate assert_precondition in favour of two methods; assert_implements (which maps to FAIL) and assert_implements_optional (which maps to PRECONDITION_FAILED for legacy reasons).

@stephenmcgruer stephenmcgruer self-assigned this Mar 30, 2020
@clopez
Copy link
Contributor Author

clopez commented Mar 31, 2020

Are these areas of the spec OPTIONAL?

I don't think this are optional.

See web-platform-tests/rfcs#48 for an in-progress RFC where we intend to deprecate assert_precondition in favour of two methods; assert_implements (which maps to FAIL) and assert_implements_optional (which maps to PRECONDITION_FAILED for legacy reasons).

Interesting, i'll wait for that to land and then update this PR to use assert_implements

@stephenmcgruer
Copy link
Contributor

FYI assert_implements and assert_implements_optional are now available in WPT.

@clopez
Use assert_implements in test the-input-element/type-change-state.html
c9d9deb 
* Use assert_implements to check if the browser supports both input
element types before trying to change the state from one to other.

* For example, WebKitGTK and Safari doesn't support several of the input
types tested here (like those related to dates). The output of the test
its clearer if the precondition its checked before.
@clopez clopez force-pushed the pr_assert_precondition_type-change-state branch from b2df59a to c9d9deb Compare April 22, 2020 14:30
@clopez
Copy link
Contributor Author

clopez commented Apr 22, 2020

Updated PR to use assert_implements

@stephenmcgruer stephenmcgruer changed the title Use assert_precondition in test the-input-element/type-change-state.html Use assert_implements in test the-input-element/type-change-state.html Apr 22, 2020
annevk
annevk approved these changes Apr 23, 2020
View reviewed changes
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine. Consider using const in future PRs.

@clopez clopez merged commit af8fb74 into web-platform-tests:master Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annevk annevk annevk approved these changes

@stephenmcgruer stephenmcgruer Awaiting requested review from stephenmcgruer

@domenic domenic Awaiting requested review from domenic

@foolip foolip Awaiting requested review from foolip

@jdm jdm Awaiting requested review from jdm

@jgraham jgraham Awaiting requested review from jgraham

@tkent-google tkent-google Awaiting requested review from tkent-google

@zcorpan zcorpan Awaiting requested review from zcorpan

@zqzhang zqzhang Awaiting requested review from zqzhang

Assignees

@annevk annevk

Labels
html
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@clopez @stephenmcgruer @annevk @wpt-pr-bot